Skip to content

feat(api): make photo storage quota reservation race-safe - #41

Merged
MehrshadFb merged 4 commits into
mainfrom
feat/race-safe-storage-quota
Sep 3, 2026
Merged

MehrshadFb merged 4 commits into
mainfrom
feat/race-safe-storage-quota

Conversation

@MehrshadFb

@MehrshadFb MehrshadFb commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add PhotoStorageService.reserveUploadBytes, which runs the quota check (SUM(sizeBytes) for PENDING + READY) and photo.createMany in one Prisma transaction at Serializable isolation
  • Retry reservations that lose a serialization conflict up to 5 times with jittered linear backoff (25 ms × attempt + jitter), then fail with 409 STORAGE_RESERVATION_CONFLICT
  • Recognise both shapes Prisma produces for SQLSTATE 40001: P2034 when a statement inside the callback fails, and the raw driver adapter error when COMMIT fails
  • Put that recognition in src/prisma/prisma.errors.ts rather than in the photo service, with its own unit tests
  • Move sleep and the jittered linear backoff into src/common/utils/async.utils.ts, also with their own tests
  • Mint presigned S3 URLs only after the transaction commits, so no DB transaction is held open across S3 calls
  • Keep assertCanUpload as a read-only pre-check; document the design, the measured retry budget, and the advisory-lock alternative in photos-architecture.md

Why

The quota check and the PENDING insert in POST /events/:eventId/photos/upload-urls were two statements outside a transaction. Two concurrent calls for the same uploader (or one client spread across two API instances) could both read usage below the cap and both insert, overshooting PHOTO_STORAGE_LIMIT_BYTES. Under Serializable isolation Postgres commits one overlapping reservation per round and aborts the rest, which re-read usage on retry.

Notes

  • Five attempts cleared bursts of eight parallel in-quota batches without a 409 against Postgres 16; three attempts started giving up at four
  • Lost rounds are logged as photo.storage.reservation_conflict with the attempt count
  • GET /users/me/storage is unchanged
  • No OpenAPI or mobile changes; the endpoint's request and success response shapes are unchanged

API

  • 409 when a reservation still conflicts after all retries (Storage reservation conflicted with a concurrent upload, please retry)
  • 413 when quota exceeded (unchanged)

Notes on what moved out of the service

isSerializationFailure is Prisma and Postgres dialect, not photo logic, so it now lives beside PrismaService in the layer that already owns the adapter. A feature module should not have to learn that the same failure arrives mapped as P2034 from inside a transaction callback but unmapped from the driver adapter at COMMIT.

That drops about 20 lines from photo-storage.service.ts and gives the next predicate an obvious home. Three read-then-write uniqueness checks already exist in the codebase (resolveByProviderSub, assertEmailIsUnique, joinByInvitationUrl) that should catch P2002 instead of pre-checking and hoping. That fix belongs in its own PR, but it will land in this file when it comes.

sleep and jitteredLinearBackoffMs moved to src/common/utils/async.utils.ts. The backoff takes its base delay as an argument rather than reading STORAGE_RESERVATION_RETRY_DELAY_MS directly, so it is callable by anything, and the tuning constant stays in photos.constants.ts where the measurement that justifies it lives. Its tests pin the linear growth, the jitter window, and the property the jitter exists for: that concurrent losers of the same round do not wake together.

The retry loop itself deliberately stays in the photo service. The attempt budget and the choice to surface 409 are quota decisions, and one caller is not enough to design a general retry wrapper around. Between them these three commits take photo-storage.service.ts from 160 lines to 134, all of it removal of things that were never about photos.

Test plan

  • npx jest photo-storage photos.service --watchman=false
  • npx jest --config ./test/jest-e2e.json photos.e2e --watchman=false
  • npm run typecheck
  • CI passes on PR

Check-then-insert was two statements outside a transaction, so two
concurrent upload-urls calls for the same uploader (or one client
spread across two API instances) could both read usage below the cap
and both insert, overshooting PHOTO_STORAGE_LIMIT_BYTES.

- Add PhotoStorageService.reserveUploadBytes: SUM(sizeBytes) over the
  caller's PENDING/READY photos and photo.createMany now run in one
  Prisma transaction at Serializable isolation, so Postgres commits one
  overlapping reservation per round and aborts the rest, which re-read
  usage on retry
- Retry aborted reservations up to 5 times with jittered linear
  backoff, then fail with 409 STORAGE_RESERVATION_CONFLICT; recognise
  both shapes Prisma produces for SQLSTATE 40001 (P2034 when a
  statement in the callback fails, the raw driver adapter error when
  COMMIT fails)
- Mint presigned S3 URLs only after the transaction commits so no DB
  transaction is held open across S3 calls
- Keep assertCanUpload as a read-only pre-check; GET /users/me/storage
  is unchanged
- Cover the transaction path, retries, and 409 in unit and e2e tests;
  document the design, the measured retry budget, and the advisory
  lock alternative in photos-architecture.md
@MehrshadFb
MehrshadFb force-pushed the feat/race-safe-storage-quota branch from 27a6686 to b18ccbb Compare September 2, 2026 16:00
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

API unit-test coverage

Metric Coverage
Statements 67.68% (578/854)
Branches 50% (239/478)
Functions 72.54% (111/153)
Lines 65.79% (504/766)

Unit suite only; controllers are exercised by the e2e suite.

@MehrshadFb
MehrshadFb merged commit 2762b5e into main Sep 3, 2026
4 checks passed
@MehrshadFb
MehrshadFb deleted the feat/race-safe-storage-quota branch September 9, 2026 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant